Fix blkif and netif backend teardown -- do not remove devices from
authorkaf24@firebug.cl.cam.ac.uk <kaf24@firebug.cl.cam.ac.uk>
Wed, 10 May 2006 12:27:17 +0000 (13:27 +0100)
committerkaf24@firebug.cl.cam.ac.uk <kaf24@firebug.cl.cam.ac.uk>
Wed, 10 May 2006 12:27:17 +0000 (13:27 +0100)
sysfs (and hence trigger hotplug callbacks) until the devices really
are dead. This fixes a bug where the deferred code to free a blk
device was running concurrently with a hotplug-remove callback which
would try to reclaim the underlying storage. In some cases the race
would be lost and the hotplug script would fail.

Thanks to the Zhu Han at Intel for finding the root cause of this
long-term and annoying bug!

Signed-off-by: Keir Fraser <keir@xensource.com>
linux-2.6-xen-sparse/drivers/xen/blkback/common.h
linux-2.6-xen-sparse/drivers/xen/blkback/interface.c
linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c
linux-2.6-xen-sparse/drivers/xen/netback/common.h
linux-2.6-xen-sparse/drivers/xen/netback/interface.c
linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c

index a32466e3a793e8776891034f7664b831f2a12e90..4491769264ee7576e0d41ee4e7156445dd169784 100644 (file)
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/blkdev.h>
 #include <linux/vmalloc.h>
+#include <linux/wait.h>
 #include <asm/io.h>
 #include <asm/setup.h>
 #include <asm/pgalloc.h>
@@ -90,21 +91,21 @@ typedef struct blkif_st {
        int                 st_wr_req;
        int                 st_oo_req;
 
-       struct work_struct free_work;
+       wait_queue_head_t waiting_to_free;
 
        grant_handle_t shmem_handle;
        grant_ref_t    shmem_ref;
 } blkif_t;
 
-blkif_t *alloc_blkif(domid_t domid);
-void free_blkif_callback(blkif_t *blkif);
+blkif_t *blkif_alloc(domid_t domid);
+void blkif_free(blkif_t *blkif);
 int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn);
 
 #define blkif_get(_b) (atomic_inc(&(_b)->refcnt))
 #define blkif_put(_b)                                  \
        do {                                            \
                if (atomic_dec_and_test(&(_b)->refcnt)) \
-                       free_blkif_callback(_b);        \
+                       wake_up(&(_b)->waiting_to_free);\
        } while (0)
 
 /* Create a vbd. */
index cc5c6b5930a0cbd5f9f9951b637e0ea13751bdcc..e2952ae1439bfb15dee1ecad780a5c94dbbb2caf 100644 (file)
@@ -35,7 +35,7 @@
 
 static kmem_cache_t *blkif_cachep;
 
-blkif_t *alloc_blkif(domid_t domid)
+blkif_t *blkif_alloc(domid_t domid)
 {
        blkif_t *blkif;
 
@@ -49,6 +49,7 @@ blkif_t *alloc_blkif(domid_t domid)
        atomic_set(&blkif->refcnt, 1);
        init_waitqueue_head(&blkif->wq);
        blkif->st_print = jiffies;
+       init_waitqueue_head(&blkif->waiting_to_free);
 
        return blkif;
 }
@@ -138,33 +139,25 @@ int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn)
        return 0;
 }
 
-static void free_blkif(void *arg)
+void blkif_free(blkif_t *blkif)
 {
-       blkif_t *blkif = (blkif_t *)arg;
+       atomic_dec(&blkif->refcnt);
+       wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
 
        /* Already disconnected? */
-       if (blkif->irq) {
+       if (blkif->irq)
                unbind_from_irqhandler(blkif->irq, blkif);
-               blkif->irq = 0;
-       }
 
        vbd_free(&blkif->vbd);
 
        if (blkif->blk_ring.sring) {
                unmap_frontend_page(blkif);
                free_vm_area(blkif->blk_ring_area);
-               blkif->blk_ring.sring = NULL;
        }
 
        kmem_cache_free(blkif_cachep, blkif);
 }
 
-void free_blkif_callback(blkif_t *blkif)
-{
-       INIT_WORK(&blkif->free_work, free_blkif, (void *)blkif);
-       schedule_work(&blkif->free_work);
-}
-
 void __init blkif_interface_init(void)
 {
        blkif_cachep = kmem_cache_create("blkif_cache", sizeof(blkif_t), 
index 011bec8d19a6c69ea0ebb0219305a46f2ac4d0f1..252865f527d49e0c81e906ebd9351767b56be2a9 100644 (file)
@@ -108,7 +108,7 @@ static int blkback_remove(struct xenbus_device *dev)
        if (be->blkif) {
                if (be->blkif->xenblkd)
                        kthread_stop(be->blkif->xenblkd);
-               blkif_put(be->blkif);
+               blkif_free(be->blkif);
                be->blkif = NULL;
        }
 
@@ -140,7 +140,7 @@ static int blkback_probe(struct xenbus_device *dev,
        be->dev = dev;
        dev->data = be;
 
-       be->blkif = alloc_blkif(dev->otherend_id);
+       be->blkif = blkif_alloc(dev->otherend_id);
        if (IS_ERR(be->blkif)) {
                err = PTR_ERR(be->blkif);
                be->blkif = NULL;
index e328b63e54bfb6a951dfb7123e1dcfd41f32660d..caa8b3510912af66d60792ea6266a1c5a362d70a 100644 (file)
@@ -38,6 +38,7 @@
 #include <linux/in.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
+#include <linux/wait.h>
 #include <xen/evtchn.h>
 #include <xen/interface/io/netif.h>
 #include <asm/io.h>
@@ -91,7 +92,7 @@ typedef struct netif_st {
        struct net_device *dev;
        struct net_device_stats stats;
 
-       struct work_struct free_work;
+       wait_queue_head_t waiting_to_free;
 } netif_t;
 
 #define NET_TX_RING_SIZE __RING_SIZE((netif_tx_sring_t *)0, PAGE_SIZE)
@@ -99,8 +100,7 @@ typedef struct netif_st {
 
 void netif_disconnect(netif_t *netif);
 
-netif_t *alloc_netif(domid_t domid, unsigned int handle, u8 be_mac[ETH_ALEN]);
-void free_netif(netif_t *netif);
+netif_t *netif_alloc(domid_t domid, unsigned int handle, u8 be_mac[ETH_ALEN]);
 int netif_map(netif_t *netif, unsigned long tx_ring_ref,
              unsigned long rx_ring_ref, unsigned int evtchn);
 
@@ -108,7 +108,7 @@ int netif_map(netif_t *netif, unsigned long tx_ring_ref,
 #define netif_put(_b)                                          \
        do {                                                    \
                if ( atomic_dec_and_test(&(_b)->refcnt) )       \
-                       free_netif(_b);                         \
+                       wake_up(&(_b)->waiting_to_free);        \
        } while (0)
 
 void netif_xenbus_init(void);
index 10095a6abf79ee8b1ca271d9446237751e43393c..2e50b7cbeacf1d33c88e2335fbd3eaff2e3c0a40 100644 (file)
@@ -78,7 +78,7 @@ static struct ethtool_ops network_ethtool_ops =
        .set_tx_csum = ethtool_op_set_tx_csum,
 };
 
-netif_t *alloc_netif(domid_t domid, unsigned int handle, u8 be_mac[ETH_ALEN])
+netif_t *netif_alloc(domid_t domid, unsigned int handle, u8 be_mac[ETH_ALEN])
 {
        int err = 0, i;
        struct net_device *dev;
@@ -97,7 +97,8 @@ netif_t *alloc_netif(domid_t domid, unsigned int handle, u8 be_mac[ETH_ALEN])
        netif->domid  = domid;
        netif->handle = handle;
        netif->status = DISCONNECTED;
-       atomic_set(&netif->refcnt, 0);
+       atomic_set(&netif->refcnt, 1);
+       init_waitqueue_head(&netif->waiting_to_free);
        netif->dev = dev;
 
        netif->credit_bytes = netif->remaining_credit = ~0UL;
@@ -273,9 +274,10 @@ err_rx:
        return err;
 }
 
-static void free_netif_callback(void *arg)
+static void netif_free(netif_t *netif)
 {
-       netif_t *netif = (netif_t *)arg;
+       atomic_dec(&netif->refcnt);
+       wait_event(netif->waiting_to_free, atomic_read(&netif->refcnt) == 0);
 
        if (netif->irq)
                unbind_from_irqhandler(netif->irq, netif);
@@ -291,12 +293,6 @@ static void free_netif_callback(void *arg)
        free_netdev(netif->dev);
 }
 
-void free_netif(netif_t *netif)
-{
-       INIT_WORK(&netif->free_work, free_netif_callback, (void *)netif);
-       schedule_work(&netif->free_work);
-}
-
 void netif_disconnect(netif_t *netif)
 {
        switch (netif->status) {
@@ -308,10 +304,9 @@ void netif_disconnect(netif_t *netif)
                        __netif_down(netif);
                rtnl_unlock();
                netif_put(netif);
-               break;
+               /* fall through */
        case DISCONNECTED:
-               BUG_ON(atomic_read(&netif->refcnt) != 0);
-               free_netif(netif);
+               netif_free(netif);
                break;
        default:
                BUG();
index d270f099c4767f104ebbfd56a56a81087d84ed81..6ba606c5f1c0d139a48a86b5dcfd84896652b822 100644 (file)
@@ -172,7 +172,7 @@ static void backend_changed(struct xenbus_watch *watch,
        if (be->netif == NULL) {
                u8 be_mac[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
 
-               be->netif = alloc_netif(dev->otherend_id, handle, be_mac);
+               be->netif = netif_alloc(dev->otherend_id, handle, be_mac);
                if (IS_ERR(be->netif)) {
                        err = PTR_ERR(be->netif);
                        be->netif = NULL;